Skip to content

[cpyrt] Detect enum arguments by type in overload priority - #75

Merged
aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:enum-args-by-type-priority
Sep 30, 2026
Merged

aaronj0 merged 1 commit into
compiler-research:mainfrom
aaronj0:enum-args-by-type-priority

Conversation

@aaronj0

@aaronj0 aaronj0 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

GetPriority() deprioritizes enum arguments so that a competing integer overload wins, since C++ has no implicit int->enum conversion, but it detected them with IsEnumScope(GetScope(name)). GetScope does not resolve an enum name to a scope, so the penalty never applied. Query the argument type with IsEnumType as well, per the existing FIXME.

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this goes in the right direction! pick(2) now returns 2, penalty fires via IsEnumType(GetMethodArgType(...)). One remaining problem though: the penalty is argument-agnostic, so pick(Color.Green) now returns 2: previously 1, and C++ overload resolution would also give 1. An enum object is an exact match for the enum overload.

I think this is a behavioral regression that needs to be fixed and locked in to the unit test.

@aaronj0
aaronj0 force-pushed the enum-args-by-type-priority branch 2 times, most recently from 2002629 to 90724f7 Compare September 24, 2026 12:17
@aaronj0

aaronj0 commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks, this goes in the right direction! pick(2) now returns 2, penalty fires via IsEnumType(GetMethodArgType(...)). One remaining problem though: the penalty is argument-agnostic, so pick(Color.Green) now returns 2: previously 1, and C++ overload resolution would also give 1. An enum object is an exact match for the enum overload.

I think this is a behavioral regression that needs to be fixed and locked in to the unit test.

Makes sense. Since the priority alone can't tell the two calls apart, I've addressed this distinction by moving it into the converter logic itself. By-value enum parameters now get an EnumConverter that takes an instance of the same enum as an exact match in the first round and delegates the actual conversion to the underlying integer converter, while the integer converters defer enum instances to the implicit round, as they already do for bools. So now in our test example, pick(Color.Green) reaches the enum overload before the integer one is allowed to accept it, pick(2) still goes to pick(unsigned int), and an enum passed to a plain integer parameter still converts when there is no enum overload.

GetPriority() deprioritizes enum arguments so that a competing integer
overload wins, since C++ has no implicit int->enum conversion, but it
detected them with IsEnumScope(GetScope(name)). GetScope does not
resolve an enum name to a scope, so the penalty never applied. Query the
argument type with IsEnumType as well, per the existing FIXME.

The priority is argument-agnostic, so on its own it also sends an enum
instance to the integer overload. Give by-value enum parameters a
converter that accepts an instance of the same enum as an exact match in
the first resolution round, and have the integer converters defer enum
instances to the implicit round, as they already do for bools.
@aaronj0
aaronj0 force-pushed the enum-args-by-type-priority branch from 90724f7 to 7d36505 Compare September 30, 2026 07:58

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for addressing my change request

@aaronj0
aaronj0 merged commit c7f67ce into compiler-research:main Sep 30, 2026
24 of 25 checks passed
aaronj0 added a commit that referenced this pull request Oct 1, 2026
#75 defers enum instances out of the exact round for integer parameters
so that an enum overload wins over an integer one. Two cases regress: an
implicit construction from the argument runs under kNoImplicit, where the
deferred enum -> int step is refused (RooFit::LineColor(kRed) with a
TColorNumber(Int_t) parameter), and in the implicit round a class overload
constructible from an int-like object is tried before the integer one
(TStyle::SetPalette(kGreyScale) picks the TString overload). Let enum
instances through the integer gate under kNoImplicit, and resolve
overloads in three stages as C++ ranks them: exact matches, standard
conversions (kNoImplicit), implicit conversions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants